Disallow unquoted literals in `LIKE` clause in `DESCRIBE` statement by Yury-Fridlyand · Pull Request #154 · Bit-Quill/opensearch-project-sql

Yury-Fridlyand · 2022-11-03T01:53:39Z

Signed-off-by: Yury-Fridlyand yuryf@bitquilltech.com

Description

Update SQL parser to disallow unquoted strings as identifiers wildcards clause.

To test this fix for opensearch-project#650 disable legacy engine first:

diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java b/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.>
index ab146404f..29c501117 100644
--- a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java
+++ b/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java
@@ -161,10 +161,11 @@ public class RestSqlAction extends BaseRestHandler {
                             QueryContext.getRequestId());
                         result.accept(channel);
                     } else {
-                        LOG.debug("[{}] Request {} is not supported and falling back to old SQL engine",
+                        LOG.info("[{}] Request {} is not supported and falling back to nowhere",
                             QueryContext.getRequestId(), newSqlRequest);
-                        QueryAction queryAction = explainRequest(client, sqlRequest, format);
-                        executeSqlRequest(request, queryAction, client, channel);
+                        throw new SQLFeatureDisabledException("buuuu");
+                        //QueryAction queryAction = explainRequest(client, sqlRequest, format);
+                        //executeSqlRequest(request, queryAction, client, channel);
                     }
                 } catch (Exception e) {
                     logAndPublishMetrics(e);
(

Before
With patch, without fix

opensearchsql> DESCRIBE TABLES LIKE flights COLUMNS LIKE time%;
{'reason': 'Invalid SQL query', 'details': 'buuuu', 'type': 'SQLFeatureDisabledException'}
opensearchsql> DESCRIBE TABLES LIKE flights COLUMNS LIKE %time%;
{'reason': 'Invalid SQL query', 'details': 'buuuu', 'type': 'SQLFeatureDisabledException'}
opensearchsql> DESCRIBE TABLES LIKE calcs;
Output longer than terminal width
Do you want to display data vertically for better visual effect? [y/N]

After

opensearchsql> DESCRIBE TABLES LIKE flights COLUMNS LIKE time%;
{'reason': 'Invalid SQL query', 'details': 'buuuu', 'type': 'SQLFeatureDisabledException'}
opensearchsql> DESCRIBE TABLES LIKE flights COLUMNS LIKE %time%;
{'reason': 'Invalid SQL query', 'details': 'buuuu', 'type': 'SQLFeatureDisabledException'}
opensearchsql> DESCRIBE TABLES LIKE calcs;
{'reason': 'Invalid SQL query', 'details': 'buuuu', 'type': 'SQLFeatureDisabledException'}

Another queries for test:

DESCRIBE TABLES LIKE acc%;
SHOW TABLES LIKE %;

and

DESCRIBE TABLES LIKE 'acc%';
SHOW TABLES LIKE "%";

Notes

As discussed, we should prohibit usage of raw identifies in SQL queries where wildcards are allowed. A user should specify string literal instead (a quoted string with ' or "). Backtick quote ` is not accepted.
This behavior was copied from legacy engine and it is an erroneous approach.
The root cause of opensearch-project#650 is a keywordsCanBeId which was in the user query which cased V2 parser to fail and fall back to V1.

Issues Resolved

opensearch-project#650

Check List

New functionality includes testing.
- All tests pass, including unit test, integration test and doctest
New functionality has been documented.
- New functionality has javadoc added
- New functionality has user manual doc added
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Yury-Fridlyand <yuryf@bitquilltech.com>

MitchellGale

LGTM :D

docs/user/dql/metadata.rst

forestmvey · 2022-11-09T23:23:43Z

FYI you spelled unquoted wrong in the branch names.

forestmvey

Looks good. FYI you spelled unquoted wrong in the branch names.

Yury-Fridlyand · 2022-11-09T23:29:34Z

FYI you spelled unquoted wrong in the branch names.

D'oh, again. I renamed integ branch, the dev one will be deleted after merge.

GumpacG · 2022-11-14T19:35:03Z

sql/src/test/java/org/opensearch/sql/sql/parser/AstBuilderTest.java

-  }
-
-  @Test
-  public void describe_compatible_with_old_engine_syntax() {


Why remove the describe test case?

Because I made it incompatible

Yury-Fridlyand · 2022-11-15T03:17:37Z

PPL does not require update. DESCRIBE command has the same syntax as source=, changing the syntax is unacceptable.

Syntax fix in legacy engine is more complicated, because parsing of the query performed not by ANTLR.

opensearch-project-sql/legacy/src/main/java/org/opensearch/sql/legacy/domain/IndexStatement.java

Lines 26 to 50 in 2b918a6

    
           private void parseQuery() { 
        
               String[] statement = query.split(" "); 
        
               int tokenLength = statement.length; 
        
               try { 
        
                   for (int i = 1; i < tokenLength; i++) { 
        
                       switch (statement[i].toUpperCase()) { 
        
                           case "TABLES": 
        
                               if (i + 1 < tokenLength && statement[i + 1].equalsIgnoreCase("LIKE")) { 
        
                                   if (i + 2 < tokenLength) { 
        
                                       indexPattern = replaceWildcard(statement[i + 2]); 
        
                                       i += 2; 
        
                                   } 
        
                               } 
        
                               break; 
        
                           case "COLUMNS": 
        
                               if (i + 1 < tokenLength && statement[i + 1].equalsIgnoreCase("LIKE")) { 
        
                                   if (i + 2 < tokenLength) { 
        
                                       columnPattern = replaceWildcard(statement[i + 2]); 
        
                                       i += 2; 
        
                                   } 
        
                               } 
        
                               break; 
        
                       } 
        
                   }

MaxKsyunz

I'd make it clear in the title that only DESCRIBE statement is affected.

* Import h3 library (#154) Made following changes to make it compatible: 1. Rename package from elasticsearch to opensearch.geospatial 2. Update License headers 3. Update build file 4. Update settings to include sub projects * Use Transport Request (#164) Remove usage of deprecated BaseNodeRequest * Update http client package to resolve build failure (#168) (#171) * Introduce H3 min resolution constant (#165) H3 version 1 has 16 resolutions, numbered 0 through 15. Introduced a constant to represent min value, similar to max value. * Add geohex aggregation (#160) This aggregation will use uber's h3 to group coordinates into H3 cell. Created new aggregation type geohex_grid. The precision will be between 0 and 15. This aggreation has default precision as 5, similar to geohash and geotile. Signed-off-by: Vijayan Balasubramanian <balasvij@amazon.com> * Add integration test (#176) Included integration test for geohex_grid. Signed-off-by: Vijayan Balasubramanian <balasvij@amazon.com>

Yury-Fridlyand changed the title ~~Update SQL parser rules.~~ [PoC] Allow unquoted percent sign in LIKE clause Nov 3, 2022

This comment was marked as abuse.

Sign in to view

Yury-Fridlyand mentioned this pull request Nov 8, 2022

SQL ingestion and ops - standup opensearch-project/sql#791

Closed

Allow quoted literals only in DESCRIBE and SHOW clauses. Tests.

0ec464a

Signed-off-by: Yury-Fridlyand <yuryf@bitquilltech.com>

Yury-Fridlyand force-pushed the dev-unqouted-percent-in-like-clause branch from 9eafb4a to 0ec464a Compare November 9, 2022 05:46

Yury-Fridlyand changed the title ~~[PoC] Allow unquoted percent sign in LIKE clause~~ [PoC] Disallow unquoted literals in LIKE clause Nov 9, 2022

Yury-Fridlyand changed the title ~~[PoC] Disallow unquoted literals in LIKE clause~~ Disallow unquoted literals in LIKE clause Nov 9, 2022

Yury-Fridlyand marked this pull request as ready for review November 9, 2022 16:54

Yury-Fridlyand requested review from GumpacG, MaxKsyunz, MitchellGale, acarbonetto, forestmvey and margarit-h November 9, 2022 16:54

MitchellGale approved these changes Nov 9, 2022

View reviewed changes

forestmvey reviewed Nov 9, 2022

View reviewed changes

docs/user/dql/metadata.rst Show resolved Hide resolved

forestmvey approved these changes Nov 9, 2022

View reviewed changes

GumpacG reviewed Nov 14, 2022

View reviewed changes

GumpacG approved these changes Nov 14, 2022

View reviewed changes

MaxKsyunz approved these changes Dec 15, 2022

View reviewed changes

Yury-Fridlyand changed the title ~~Disallow unquoted literals in LIKE clause~~ Disallow unquoted literals in LIKE clause in DESCRIBE statement Dec 15, 2022

Yury-Fridlyand merged commit 2d118a1 into integ-unquoted-percent-in-like-clause Dec 15, 2022

Yury-Fridlyand deleted the dev-unqouted-percent-in-like-clause branch December 15, 2022 20:10

Yury-Fridlyand mentioned this pull request Dec 15, 2022

Disallow unquoted literals in LIKE clause in DESCRIBE statement opensearch-project/sql#1181

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disallow unquoted literals in `LIKE` clause in `DESCRIBE` statement#154

Disallow unquoted literals in `LIKE` clause in `DESCRIBE` statement#154
Yury-Fridlyand merged 1 commit intointeg-unquoted-percent-in-like-clausefrom
dev-unqouted-percent-in-like-clause

Yury-Fridlyand commented Nov 3, 2022 •

edited

Loading

Uh oh!

This comment was marked as abuse.

MitchellGale left a comment

Uh oh!

Uh oh!

forestmvey commented Nov 9, 2022

Uh oh!

forestmvey left a comment

Uh oh!

Yury-Fridlyand commented Nov 9, 2022

Uh oh!

GumpacG Nov 14, 2022

Uh oh!

Yury-Fridlyand Nov 15, 2022

Uh oh!

Yury-Fridlyand commented Nov 15, 2022 •

edited

Loading

Uh oh!

MaxKsyunz left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

Yury-Fridlyand commented Nov 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Notes

Issues Resolved

Check List

Uh oh!

This comment was marked as abuse.

MitchellGale left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

forestmvey commented Nov 9, 2022

Uh oh!

forestmvey left a comment

Choose a reason for hiding this comment

Uh oh!

Yury-Fridlyand commented Nov 9, 2022

Uh oh!

GumpacG Nov 14, 2022

Choose a reason for hiding this comment

Uh oh!

Yury-Fridlyand Nov 15, 2022

Choose a reason for hiding this comment

Uh oh!

Yury-Fridlyand commented Nov 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MaxKsyunz left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Yury-Fridlyand commented Nov 3, 2022 •

edited

Loading

Yury-Fridlyand commented Nov 15, 2022 •

edited

Loading